Skip to content

Conversation

Zeophlite
Copy link
Contributor

Objective

Solution

  • Describe the solution used to achieve the objective above.
  • Added an example that demos picking in a heirachy

Testing

  • Ran cargo build and cargo test --all-targets
  • Ran directional_navigation example

@ickshonpe ickshonpe added A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change A-Pointers Relating to Bevy pointer abstractions D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Apr 29, 2025
@chescock
Copy link
Contributor

I think #18710 is trying to tackle this same confusion, while also making the "original target" available for all events instead of just Pointer events. It's also proposing a different naming convention of target vs current_target. I don't know what the right answer is here, but it might help to combine the discussions!

@alice-i-cecile
Copy link
Member

Sorry! I didn't see this, and accidentally duplicated it in #19609. Your review there would be very welcome. I don't think the example is particularly essential, so I'm going to close this out, but we can open a new PR with just the example if you feel otherwise.

github-merge-queue bot pushed a commit that referenced this pull request Jun 15, 2025
…19663)

# Objective

Getting access to the original target of an entity-event is really
helpful when working with bubbled / propagated events.

`bevy_picking` special-cases this, but users have requested this for all
sorts of bubbled events.

The existing naming convention was also very confusing. Fixes
#17112, but also see #18982.

## Solution

1. Rename `ObserverTrigger::target` -> `current_target`.
1. Store `original_target: Option<Entity>` in `ObserverTrigger`.
1. Wire it up so this field gets set correctly.
1. Remove the `target` field on the `Pointer` events from
`bevy_picking`.

Closes #18710, which attempted
the same thing. Thanks @emfax!

## Testing

I've modified an existing test to check that the entities returned
during event bubbling / propagation are correct.

## Notes to reviewers

It's a little weird / sad that you can no longer access this infromation
via the buffered events for `Pointer`. That said, you already couldn't
access any bubbled target. We should probably remove the `BufferedEvent`
form of `Pointer` to reduce confusion and overhead, but I didn't want to
do so here.

Observer events can be trivially converted into buffered events (write
an observer with an EventWriter), and I suspect that that is the better
migration if you want the controllable timing or performance
characteristics of buffered events for your specific use case.

## Future work

It would be nice to not store this data at all (and not expose any
methods) if propagation was disabled. That involves more trait
shuffling, and I don't think we should do it here for reviewability.

---------

Co-authored-by: Joona Aalto <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events A-Pointers Relating to Bevy pointer abstractions C-Code-Quality A section of code that is hard to understand or change D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Target::target and Target::target() return different entities
4 participants